KBS: combine CoCo Token and Jwk Token verifier #524

Xynnn007 · 2024-10-09T04:18:03Z

This patch is part of #514.

ITA token and CoCo Token are both JWTs. They both need a JWK to verify the JWT. The difference is the way to gather the JWK.

This commit combined the two logic, and add two ways to get the JWK.

From the configured JwkSet when launching KBS
From the JWT's Header's jwk field.

The two ways will check the jwk endorsement in different ways. The first way is to configure the trusted JwkSet from the config. The second way is to configure the trusted CA in config. Then get the public key cert chain from Jwk's x5c field. The both ways are also supported in this patch.

The difference between CoCo Token and ITA Token are where Tee Public key is embedded inside the token, this could be configured in the launch toml of KBS, because it is hard to detect during runtime.

Rust does not provide a mature crate to verify cert chain, thus openssl is used in this patch. We also abondon rustls and openssl feature of KBS because openssl is by default used. Then we use openssl by default to make the code base simpler.

Close #486

cc @mythi

LiuPeng-running · 2024-10-10T01:18:52Z

Good good good.

Xynnn007 · 2024-10-10T02:00:53Z

This PR also changes some logic of previous JWK of CoCoAS, so ptal @jialez0

fitzthum

LGTM

Wdyt @mythi

kbs/src/token/mod.rs

mythi · 2024-10-14T10:01:03Z

My earlier comment in the original PR was that my preference is to not expose "ITA" as a token type to the user and that seems to be in this PR still. I was wondering whether we could just ask users to provide the list of trust anchors and try to parse the provided file:// and OpenID config paths.

The only reason for "token type" to still exist seems to be the tee-pubkey search path in the claim. Would it make sense to have the paths configurable instead of having them implicit via the token type? The search paths config could be a list to support tokens with different paths simultaneously.

Xynnn007 · 2024-10-15T02:17:06Z

I was wondering whether we could just ask users to provide the list of trust anchors and try to parse the provided file:// and OpenID config paths.

Yes. This PR supports the original ITA way, s.t. provode file:// to speficy jwkset, or openid url for jwksets. The modification is the configuration name in the config file, from trusted_certs_paths to trusted_jwk_sets.

Another option is trusted_certs_paths. It's for trusted root certs to verify the jwk/x5c field within the JWT header.

The only reason for "token type" to still exist seems to be the tee-pubkey search path in the claim. Would it make sense to have the paths configurable instead of having them implicit via the token type? The search paths config could be a list to support tokens with different paths simultaneously.

Nice suggestion. The pro is flexibility while the con is extra complexity to remember the paths. I personally prefer the "token type" way as the user only needs to care about token type rather than claims inside that. I think you are caring about the name ITA to be exposed to users, so what do you prefer to name it?

mythi · 2024-10-15T05:50:20Z

The modification is the configuration name in the config file, from trusted_certs_paths to trusted_jwk_sets.

Another option is trusted_certs_paths. It's for trusted root certs to verify the jwk/x5c field within the JWT header.

I was not very clear earlier. My comment was that can we have just one config entry. It would be easier for the user. It should be possible to handle ["file:///foo/coco.pem", "file:///foo/ita.jwks"].

The pro is flexibility while the con is extra complexity to remember the paths

True, OTOH, we can always make known paths as the default and only provide the mechanism to add supplemental paths.

mythi · 2024-10-15T05:51:58Z

Would it be OK to aim fixing #519 with PR too?

Xynnn007 · 2024-10-15T06:58:44Z

Would it be OK to aim fixing #519 with PR too?

Probably not. Currently it is hard for KBS to detect the JWT type (CoCoAS/ITA or else) runtimely. See #519 (comment)

Xynnn007 · 2024-10-15T09:54:14Z

True, OTOH, we can always make known paths as the default and only provide the mechanism to add supplemental paths.

Do you mean that we have two config items

type: specify the token type, s.t. CoCo or ITA
tee_publickey_path: if this parameter is given, ignore type and try to extract tee key from this path

mythi · 2024-10-15T10:25:13Z

Would it be OK to aim fixing #519 with PR too?

Probably not. Currently it is hard for KBS to detect the JWT type (CoCoAS/ITA or else) runtimely. See #519 (comment)

Could kid/x5* header fields be used to map to what trust anchors we have configured. ITA token has kidand we use it to search the corresponding JWK in the JwkSet.

Do you mean that we have two config items

Just one because type isn't used for anything else than just for mapping to tee-pubkey paths. So I'm thinking could we drop type completely, have some default tee-pubkey paths and allow users to configure supplemental tee_publickey_paths.

Xynnn007 · 2024-10-16T06:09:26Z

Could kid/x5*header fields be used to map to what trust anchors we have configured. ITA token haskid`and we use it to search the corresponding JWK in the JwkSet.

Yes it can and it is implemented in this PR. What #519 aims is to support different public key claim path.

Just one because type isn't used for anything else than just for mapping to tee-pubkey paths. So I'm thinking could we drop type completely, have some default tee-pubkey paths and allow users to configure supplemental tee_publickey_paths.

Ok. Could you provide an example for this in your mind?

mythi · 2024-10-16T06:59:27Z

Could kid/x5*header fields be used to map to what trust anchors we have configured. ITA token haskid`and we use it to search the corresponding JWK in the JwkSet.

Yes it can and it is implemented in this PR. What #519 aims is to support different public key claim path.

Right, sorry. I had missed that and got side tracked when you wrote that JWT detection cannot be done easily.

Just one because type isn't used for anything else than just for mapping to tee-pubkey paths. So I'm thinking could we drop type completely, have some default tee-pubkey paths and allow users to configure supplemental tee_publickey_paths.

Ok. Could you provide an example for this in your mind?

Instead of match self.token_type we could just loop all configured/known paths until claim.pointer(path) returns Ok. For starters this could be just the two we know today (i.e., TOKEN_TEE_PUBKEY_PATH_COCO and TOKEN_TEE_PUBKEY_PATH_ITA) but maybe we want to add a config entry that allows users to add more paths to search.

Actually, the ITA token and CoCo Token are both JWTs. They both need a JWK to verify the JWT. The difference is the way to gather the JWK. This commit combined the two logic, and add two ways to get the JWK. 1. From the configured JwkSet when launching KBS 2. From the JWT's Header's jwk field. The two ways will check the jwk endorsement in different ways. The first way is to configure the trusted JwkSet from the config. The second way is to configure the trusted CA in config. Then get the public key cert chain from Jwk's x5c field. The both ways are also supported in this patch. Rust does not provide a mature crate to verify cert chain, thus openssl is used in this patch. We also abondon rustls and openssl feature of KBS because openssl is by default used. Then we use openssl by default to make the code base simpler. Signed-off-by: Xynnn007 <[email protected]>

Due to RFC 7515, JWK should be part of a JOSE Header rather than claim body. Signed-off-by: Xynnn007 <[email protected]>

Xynnn007 · 2024-10-16T09:21:56Z

@mythi Adds the change that you suggested. I think it is good to have some internal defined paths together with user defined extra paths.

mythi · 2024-10-16T11:08:56Z

@mythi Adds the change that you suggested. I think it is good to have some internal defined paths together with user defined extra paths.

thanks! I personally feel it's better this way and I guess this now meets #519 also. My remaining concern is whether we need separate user facing config entries for trusted_certs_paths and trusted_jwk_sets.

Xynnn007 · 2024-10-16T11:58:59Z

Well as they are different things. One is jwkset and the other is root pem certs

mythi · 2024-10-16T15:05:57Z

kbs/src/attestation/intel_trust_authority/mod.rs

+            extra_teekey_paths: vec![],
+            trusted_certs_paths: vec![],
+            trusted_jwk_sets: vec![config.certs_file.clone()],
+            insecure_key: true,


Suggested change

insecure_key: true,

insecure_key: false,

Let me fix this. Btw, do you think we need to rename certs_file to jwkset or something else?

@mythi Ok. Let me response here. Actually for ITA the configure item here both are ok as it will not change the behavior.

When I re-read the part of code, I have a question whether we need token_verifier object here inside ITA backend plugin code. I am afraid that we do not.

The logic here is

a user requests ITA for a token (and we do not need to care about the token trustiworthy as we just call the API of ITA)

the user request KBS for resource (this time we should check the token trustiworthy)

wdyt?

I had thought about the same but decided to keep it for now. We may have future feature additions like the faithful verification that further checks the token and it might be useful to do it here.

mythi · 2024-10-16T15:25:39Z

Well as they are different things. One is jwkset and the other is root pem certs

If the take this path, we might need a third one for "OpenID Configuration endpoints" which is what the default ITA config prefers. But my point was that some generic config entry would probably keep things simple for the users.

Xynnn007 · 2024-10-17T02:31:21Z

If the take this path, we might need a third one for "OpenID Configuration endpoints" which is what the default ITA config prefers. But my point was that some generic config entry would probably keep things simple for the users.

Well. There are actually three items as you said

JwkSet from local
JwkSet to get from OpenID endpoints
Root certs to verify JWKs

The 1 and 2 are in good design currently I think, by https:// or path:// prefix to distinguish
The 3 is hard to merge into 1 or 2, as it is a PEM root cert not a JWK.

Before this patch the config item trusted_certs_paths is actually used in a wrong way, at least not a good name, because it is JwkSet for ITA rather than certs.

mythi · 2024-10-17T05:17:48Z

Before this patch the config item trusted_certs_paths is actually used in a wrong way, at least not a good name, because it is JwkSet for ITA rather than certs.

FWIW, ITA documentation calls these certificates: "To verify the token signature, first download the Intel Trust Authority signing certificates from the portal. " and that's why I though reusing trusted_certs_paths is fine.

Would there be any better name describing both root certs and jwksets?

Xynnn007 · 2024-10-17T06:07:31Z

FWIW, ITA documentation calls these certificates: "To verify the token signature, first download the Intel Trust Authority signing certificates from the portal. " and that's why I though reusing trusted_certs_paths is fine.

Well, it is somehow tricky. The things download from "https://portal.trustauthority.intel.com/certs" are actually public keys with its x5cs in JWK format. This is the same as file://xxx for trusted_jwk_sets.

like

{
      "alg": "PS384",
      "e": "AQAB",
      "kid": "c68e5a7b3aec51a3ff384fe30799d465d55b16d57ef88d05f6067559d9156cd3d5302f89fe665d1fb9a6be4602be11d8",
      "kty": "RSA",
      "n": "yE07D7FRSXLsswdeK7h22kw-Xv2K2r4NFoefWElZ6FWmLvCcd27wGEczNeKrE91SWczPtR279tTasQN6_v8qsswC5rCGYlrRWvE0vuPoUXezlV4PX0tCJJJmxWtXFXW0dChWvR1j-_viOItfR8jrybV2-DyVBgGX1ad4BLJJseglPXcofhnKYcG9gp8J2zPFqs1tu6jTW-He3Xw7ZeQNq0n4ZfrRBM3GEYVVsWGlTlqVidMhbvMXSQgz1x2QjyPC2mSUrT-JyA2xTm84Mv_Lmz6FpHXsjXMyPKCUVUf8LSTAiw3UsHa-7QGUW51hh9lZsbWkdSfwGUGxjrcMNEwYo3KvcF8f9Cv1_bkla396poQhtTIHuV478PobzsCfdkbCF5CfwZN31KbqyD9o9pVyzmmQUmOikIZuiSPRnfIU_P8duM5F6yvxQPITZf1RhOPBNYLiOJge7C89OmsM46UKtAYNTieBH-J8oWUUWfAX3pO38bKIzNwHDelSbaeHterJ",
      "x5c": [
            "MIIE/DCCA2SgAwIBAgIBAjANBgkqhkiG9w0BAQ0FADBlMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExGjAYBgNVBAoMEUludGVsIENvcnBvcmF0aW9uMS0wKwYDVQQDDCRJbnRlbCBUcnVzdCBBdXRob3JpdHkgQVRTIFNpZ25pbmcgQ0EwHhcNMjQwOTA5MTAzMzQ3WhcNMjUwOTA5MTAzMzQ3WjBwMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExGjAYBgNVBAoMEUludGVsIENvcnBvcmF0aW9uMTgwNgYDVQQDDC9JbnRlbCBUcnVzdCBBdXRob3JpdHkgQXR0ZXN0YXRpb24gVG9rZW4gU2lnbmluZzCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoCggGBAMhNOw+xUUly7LMHXiu4dtpMPl79itq+DRaHn1hJWehVpi7wnHdu8BhHMzXiqxPdUlnMz7Udu/bU2rEDev7/KrLMAuawhmJa0VrxNL7j6FF3s5VeD19LQiSSZsVrVxV1tHQoVr0dY/v74jiLX0fI68m1dvg8lQYBl9WneASySbHoJT13KH4ZymHBvYKfCdszxarNbbuo01vh3t18O2XkDatJ+GX60QTNxhGFVbFhpU5alYnTIW7zF0kIM9cdkI8jwtpklK0/icgNsU5vODL/y5s+haR17I1zMjyglFVH/C0kwIsN1LB2vu0BlFudYYfZWbG1pHUn8BlBsY63DDRMGKNyr3BfH/Qr9f25JWt/eqaEIbUyB7leO/D6G87An3ZGwheQn8GTd9Sm6sg/aPaVcs5pkFJjopCGbokj0Z3yFPz/HbjOResr8UDyE2X9UYTjwTWC4jiYHuwvPTprDOOlCrQGDU4ngR/ifKFlFFnwF96Tt/GyiMzcBw3pUm2nh7XqyQIDAQABo4GrMIGoMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFGXRi1GAPlb8fBXUcGQrN5P0nrQAMB8GA1UdIwQYMBaAFCRX9lEHLr6HXZtQaFKogfoiHFl5MAsGA1UdDwQEAwIE8DBLBgNVHR8ERDBCMECgPqA8hjpodHRwczovL3BvcnRhbC50cnVzdGF1dGhvcml0eS5pbnRlbC5jb20vY3JsL2F0cy1jYS1jcmwuZGVyMA0GCSqGSIb3DQEBDQUAA4IBgQAGg5OCRX0QWAoJDhP4bdB2ECu/0WLS1XZHO0eWk7tL+358QAr1mf4Xyw6Y5RLK763Vrr62TeBuUFzex6CpmTp3z3dB8oWHL8VPTcsvnYZFSyL313/iKfemnaVZY7wCaUwI0Hc7UT/sIGY/gmBsDMCIJRIhDjSnx+oW0a6xDVMoevRM5P6jJIJHxNaM+4bV11+uNSQ9kysEXQBDyYaP9KtFlgKrY7YcPlnUwLNfjAB6/9CsyXxRw8YF3wjVly1zT27KSL5YcSKACu4Jzkwlz/m3GMkEfOV9y3IZEsiuplKniuljBOdcD73HdDNZzmgbEy8OHlbjar1FRcdHHbSE+NN+ZoWpzSx7p+q43/x1SSxift6SZGk7nykGJDD4vq5PYwk+ia/1xzKS3+2xsVS/z9Hz3NMMAlXjCUEbMn4vwQ7BpY3PtYPL5J/RhHt0plyqRj3db4jaTUPd/FTinHZBMYm4IdcxzIjuboTYYu1Q7fpCgGo7zT9xUSPjQNYjj2bfkRA=",
            "MIIFCjCCA3KgAwIBAgIBATANBgkqhkiG9w0BAQ0FADB0MSYwJAYDVQQDDB1JbnRlbCBUcnVzdCBBdXRob3JpdHkgUm9vdCBDQTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMRQwEgYDVQQHDAtTYW50YSBDbGFyYTEaMBgGA1UECgwRSW50ZWwgQ29ycG9yYXRpb24wHhcNMjMwOTEyMTExMTQ5WhcNMzYxMjMwMTExMTQ5WjBlMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExGjAYBgNVBAoMEUludGVsIENvcnBvcmF0aW9uMS0wKwYDVQQDDCRJbnRlbCBUcnVzdCBBdXRob3JpdHkgQVRTIFNpZ25pbmcgQ0EwggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQCof1PJ6PFnZ5TOyxBP8H7kyBxsAopUcMZtJAIdLZV+L+5DVMvH6E/hT4+7XX5SGYkb0R+XquyBz0PayzVrs71k8nL0MrwBIKLuIWQEcmTLC5/18Njf7QxBDZ3+uFrTOcfYcfYpfTNl2v/RvjEM6+KuDhxqLlH//buRO9eALngQQDqHq7pigrB9vVoOtpdng5Az4kjlDOOmdrNNigpEP4u4sQsqcAkSUFbofTXk8OiWtWClL6ItjosedwcabcdXSkAhf/T0QfYCcRQBOhSIblveZbaWgVXKb4S+HlM1Ft/QEtHNpuldlyI/s7+0ISAzVM8vRZU27EuPpQBUseNIrH2+DXTtpop13tozOl64o7VJmB7mwi+Zqv31NT0BucvMUdeC/bg2RSIKlV6RRomUTKtMFo3RpBi3K7+GUMbiq5GNQBNece294wHDhtgA+Bjg59IIxsHx3O9PmTgGAxmx8qAN2e9FPObTNWIHijfue6D0RkbolJd1/BGgAFcmF3pZy+cCAwEAAaOBtTCBsjASBgNVHRMBAf8ECDAGAQH/AgEAMB0GA1UdDgQWBBQkV/ZRBy6+h12bUGhSqIH6IhxZeTAfBgNVHSMEGDAWgBTzCwdViUpG9BjW2nyu+DI+d6gWETAOBgNVHQ8BAf8EBAMCAQYwTAYDVR0fBEUwQzBBoD+gPYY7aHR0cHM6Ly9wb3J0YWwudHJ1c3RhdXRob3JpdHkuaW50ZWwuY29tL2NybC9yb290LWNhLWNybC5kZXIwDQYJKoZIhvcNAQENBQADggGBADtWnJTjnCT6P5GZHS7Qz6MsrpM2IBwWcpnayTe+nV0CAqk4RJay7rupzq8gn8PllPXyWFoComrsT6HPZ80uh0JUIACOmNcO5UhwuRxML+EPmgpVVQJlz68AXf99Y1HaJxJ0aHkFSPr11XUOQ3S657QKee7RJijwcYu6rgfw6eVnYCGr7UD6SSW63D9nZLsa11v8GcIDWPdZVkyPnDVNJulAuWby/FQtZWAs4vCmxWpJYWoy303AVRzEBYoiyBRznWbed0ykyVU6TogLuezoxwH6jrZ7NeaFKrpbnD1YvI3JfP6EzPo1EqjpfumlVW99yY80mrHdr7FpIe9h9RL05utnYcoGt2VzbwN0H3ZXFPBwsBoioLX17xtSM7894w/rHdQV9wEMvxUT2Hmo+rRNu6lCQ3gDsLVXPvBd5rB3tnEY7wYu/uaLvHf01lq9/X9aTuISg63pFsqcb9oCS3hnx//b47/oHjo7yYCPhgKWHJdC5yiiv6U2NqQLeM9FtZIPuQ==",
            "MIIE2TCCA0GgAwIBAgIUZZX2XASGiYPzpZfwGNa7QFlK+3QwDQYJKoZIhvcNAQENBQAwdDEmMCQGA1UEAwwdSW50ZWwgVHJ1c3QgQXV0aG9yaXR5IFJvb3QgQ0ExCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEUMBIGA1UEBwwLU2FudGEgQ2xhcmExGjAYBgNVBAoMEUludGVsIENvcnBvcmF0aW9uMB4XDTIzMDkxMjExMTE0OFoXDTQ5MTIzMDExMTE0OFowdDEmMCQGA1UEAwwdSW50ZWwgVHJ1c3QgQXV0aG9yaXR5IFJvb3QgQ0ExCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEUMBIGA1UEBwwLU2FudGEgQ2xhcmExGjAYBgNVBAoMEUludGVsIENvcnBvcmF0aW9uMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAuQ1g1Q4MtnvTRhYITKVou5CDvB5FpfjBe8ssUKaDILwkgJlGNv66IWPQ9vhIekZQjuvHbUj9yPcrM7Hva7h7Ehlo/Sm7ZhY8AgZfGFTRJjvNU+vg/BTr5vuqUu/a54eewyzMcmxwmWhK/4cGQs7spFB346jpjSgOHgk5PgJ39PgEr5UL9SvJ1LFRuCNxZTdzyLe7K8cWvEnwGkR2RJpK9pYgzfnAWy8J0djdAKaoQxt8TOE/IwafG/0ujTeuNbzo+3wxeF6SGz56MimE1+KgraPpULaeX2tAL9lUz+ECMetNLbAyqHQwxN1jQZ/3VgpQ8qqh7Cyo4rEjpja29iRtOihBYlW0/X6TxOG1LLSuGo/N9CcSW6EzjsC1Bzakzjs4OD+JGaqqvc255p8URTxZSRSJr1xtimZK+BJoCHECsrCLvCC5UmFfTQrxkeGJ+OIDMkQFgidUw2K7kYXe9k7glKw9yyVf9C0hhqBFfPD8r+CXEA9m2u1tvR4NGMuoegMxAgMBAAGjYzBhMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFPMLB1WJSkb0GNbafK74Mj53qBYRMB8GA1UdIwQYMBaAFPMLB1WJSkb0GNbafK74Mj53qBYRMA4GA1UdDwEB/wQEAwIBBjANBgkqhkiG9w0BAQ0FAAOCAYEALTVDjOJL3XVuK25K6v/hd4z5vZOzhHK6ifnCjLWKu+CkPbKQMMMYMbYTpqm6gEcSr28dPMOF1IWG4xcJX9oJ6b3mDWj3NQNBomBI8fq0hRgz9hKF8wpFpU3UzXtyBLLL18BTCQeJecb6pwrpASVZNmR/fAv8fbR9wWdTBVFHmIrg1wvU1u2WqYSdAgQGke+WdWFIvZCKKMVAB0jX2kPHBQnAkLF3pRaVyNT4I2MCRB8cW2fbSCIARBeryiIHyGCKnDGkDK+dvPxMJ9eMidPbBQBp5t6jxoicg9X8Gw9MeOboOEOB1sIAd0S25V9fzIwwn6j61K4d2VkLf4ZnDa2VKTgmv6NFMynu+JlHVYhQ0yin+dfD63XJvBLcfLrJwK10lsVMX7xv3dB4P+DBYHtWPwrjE28j6+IjCIupuarzDcahBTbRZIAAW4eWKrA3MPQVyGikcdnciFN7kL12EtHhrSBL2jTzsScWvatPqqzIeNxDCywNEyjtGYLnLBnZnbSP"
      ]
}

But what we want to set trusted_certs_paths are certificates in PEM, like

The two things also work differently in code. The former one will be used to look up jwk by given kid in a token, while the latter will use the certificate to check the endorsement of the jwk in a token.

mythi · 2024-10-17T11:48:07Z

I agree that their content is very different. My only comment has been that for the user they are still both "token verification collateral" which is why I also asked:

Would there be any better name describing both root certs and jwksets?

If other reviewers think making them separate config entries is the right thing to do, I can live with that.

…ugins This is just a hack to be able to test the nebula_ca plugin. $ cd kbs && make cli ATTESTER=snp-attester && make install-cli $ docker compose up $ kbs-client config --auth-private-key kbs/config/private.key set-resource-policy --policy-file kbs/sample_policies/allow_all.rego $ kbs-client get-resource --plugin-name "nebula_ca" --resource-path "credential?ip[ip]=10.9.8.2&ip[netbits]=21&name=podA" Currently, the last command is failing reporting Error: request unauthorized and in the trustee log: ERROR kbs::error] TokenVerifierError(TokenVerificationFailed { source: Cannot verify token since trusted JWK Set is empty }) I did not get to the bottom of the problem yet, but I think I may need the PR confidential-containers#524 as well

Xynnn007 · 2024-10-21T01:56:05Z

To make it clear. The current token verifier config looks like

...
[attestation_token_config]
# Path of root certificate used to verify the trustworthy of `x5c` extension in the JWT
# Defaults to empty
trusted_certs_paths = ["/path/to/trusted_cacert.pem"]

# URL (`path://` or `https://`) of the trusted JWK that can be indexed by `kid` in
# JWT Header.
# Defaults to empty
trusted_jwk_sets = ["/url/to/trusted_jwk_set"]

# Each JWT contains a TEE Public Key. Users can use the `extra_teekey_paths` field to additionally specify the path of this Key in the JWT.
# Example of `extra_teekey_paths` is `/attester_runtime_data/tee-pubkey` which refers to the key
# `attester_runtime_data.tee-pubkey` inside the JWT body claims. By default CoCo AS Token and Intel TA
# Token TEE Public Key paths are supported.
# Defaults to empty
extra_teekey_paths = [""]

# For Attestation Services like CoCo-AS, the public key to verify the JWT will be given
# in the token's `jwk` field (with or without the public key cert chain `x5c`).
#
# - If this flag is set to `true`, KBS will ignore to verify the trustworthy of the `jwk`.
# - If this flag is set to `false`, KBS will look up its `trusted_certs_paths` and the `x5c`
# field to verify the trustworthy of the `jwk`.
# Defaults to `false`
insecure_key = false

Due to latest change, KBS will not maintain both rustls and openssl suites for HTTPS. Thus we need to delete all the options of HTTPS_CRYPTO config in documents and codes. Also, the latest change changes the config format of `attestation_token_config`, this patch also applies the change. Signed-off-by: Xynnn007 <[email protected]>

fitzthum · 2024-10-22T19:32:39Z

If other reviewers think making them separate config entries is the right thing to do, I can live with that.

I see no real benefit in having fewer config entries, especially if doing so could cause confusion. If we are dealing with different types of stuff we might as well make different optional config entries and add some logic to make sure one of them is set.

We can also tweak this down the road if we realize that it isn't ideal.

mythi · 2024-10-23T04:55:03Z

I see no real benefit in having fewer config entries, especially if doing so could cause confusion.

I was mainly thinking user experience but ok. For some users it can be confusing that data downloaded from https://portal.trustauthority.intel.com/certs don't belong to trusted_certs_paths.

jialez0

LGTM.

mythi · 2024-10-23T06:16:42Z

This PR was not ready for merging. For example, you still have insecure_key: true in place.

Xynnn007 · 2024-10-23T06:45:47Z

This PR was not ready for merging. For example, you still have insecure_key: true in place.

The configuration keeps the behavior same as before. Before this commit, the insecure key option is always true logically although there is not such a config item.

I think we can fix this in future PR.

mythi · 2024-10-23T07:08:13Z

Before this commit, the insecure key option is always true logically although there is not such a config item.

This was not the case for attestation_token_type = "Jwk" since it unconditionally expected to have JwkSet non-empty.

Xynnn007 · 2024-10-23T07:14:48Z

This was not the case for attestation_token_type = "Jwk" since it unconditionally expected to have JwkSet non-empty.

Well. For old JWK (what ITA uses), the insecure_key is not relevant to that, because ITA has a kid field in JWT. The kid is used to look up previously configured jwkset. The insecure_key only works on tokens like CoCo's, where the JWK itself is included inside the JWT.

The docs upon insecure_key is

# For Attestation Services like CoCo-AS, the public key to verify the JWT will be given
# in the token's `jwk` field (with or without the public key cert chain `x5c`).
#
# - If this flag is set to `true`, KBS will ignore to verify the trustworthy of the `jwk`.
# - If this flag is set to `false`, KBS will look up its `trusted_certs_paths` and the `x5c`
# field to verify the trustworthy of the `jwk`.

This is partially why two config items seem better to me during our previous conversion.

mythi · 2024-10-23T07:52:18Z

Fair enough, the functionality seems fine. My original comment was related to unresolved conversations that were not addressed.

Xynnn007 force-pushed the refactor-token branch from 34bc38c to 47002cd Compare October 9, 2024 04:20

Xynnn007 mentioned this pull request Oct 9, 2024

KBS | Refactoring the codebase / update config file format / bring in plugin mechanism #514

Merged

Xynnn007 force-pushed the refactor-token branch from 47002cd to 3d973d9 Compare October 9, 2024 05:13

Xynnn007 marked this pull request as ready for review October 10, 2024 02:00

Xynnn007 requested a review from a team as a code owner October 10, 2024 02:00

Xynnn007 force-pushed the refactor-token branch from 3d973d9 to 8c27e4f Compare October 10, 2024 02:04

fitzthum approved these changes Oct 11, 2024

View reviewed changes

kbs/src/token/mod.rs Show resolved Hide resolved

Xynnn007 force-pushed the refactor-token branch 3 times, most recently from 8f2d7af to 2e9d713 Compare October 16, 2024 08:57

Xynnn007 added 2 commits October 16, 2024 17:21

AS: move JWK to the JWT Header field

679a7b8

Due to RFC 7515, JWK should be part of a JOSE Header rather than claim body. Signed-off-by: Xynnn007 <[email protected]>

Xynnn007 force-pushed the refactor-token branch from 2e9d713 to 2756ca3 Compare October 16, 2024 09:21

mythi reviewed Oct 16, 2024

View reviewed changes

Xynnn007 force-pushed the refactor-token branch from 2756ca3 to edad780 Compare October 21, 2024 01:55

Xynnn007 force-pushed the refactor-token branch from edad780 to eadfd27 Compare October 21, 2024 02:05

jialez0 approved these changes Oct 23, 2024

View reviewed changes

Xynnn007 merged commit 418f5e0 into confidential-containers:main Oct 23, 2024
16 checks passed

Xynnn007 deleted the refactor-token branch October 23, 2024 06:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KBS: combine CoCo Token and Jwk Token verifier #524

KBS: combine CoCo Token and Jwk Token verifier #524

Xynnn007 commented Oct 9, 2024 •

edited

Loading

LiuPeng-running commented Oct 10, 2024

Xynnn007 commented Oct 10, 2024 •

edited

Loading

fitzthum left a comment

mythi commented Oct 14, 2024

Xynnn007 commented Oct 15, 2024

mythi commented Oct 15, 2024

mythi commented Oct 15, 2024

Xynnn007 commented Oct 15, 2024

Xynnn007 commented Oct 15, 2024

mythi commented Oct 15, 2024 •

edited

Loading

Xynnn007 commented Oct 16, 2024

mythi commented Oct 16, 2024

Xynnn007 commented Oct 16, 2024

mythi commented Oct 16, 2024

Xynnn007 commented Oct 16, 2024 •

edited

Loading

mythi Oct 16, 2024

Xynnn007 Oct 17, 2024

Xynnn007 Oct 23, 2024 •

edited

Loading

mythi Oct 23, 2024

mythi commented Oct 16, 2024

Xynnn007 commented Oct 17, 2024

mythi commented Oct 17, 2024

Xynnn007 commented Oct 17, 2024

mythi commented Oct 17, 2024

Xynnn007 commented Oct 21, 2024 •

edited

Loading

fitzthum commented Oct 22, 2024 •

edited

Loading

mythi commented Oct 23, 2024

jialez0 left a comment

mythi commented Oct 23, 2024

Xynnn007 commented Oct 23, 2024 •

edited

Loading

mythi commented Oct 23, 2024 •

edited

Loading

Xynnn007 commented Oct 23, 2024

mythi commented Oct 23, 2024

KBS: combine CoCo Token and Jwk Token verifier #524

KBS: combine CoCo Token and Jwk Token verifier #524

Conversation

Xynnn007 commented Oct 9, 2024 • edited Loading

LiuPeng-running commented Oct 10, 2024

Xynnn007 commented Oct 10, 2024 • edited Loading

fitzthum left a comment

Choose a reason for hiding this comment

mythi commented Oct 14, 2024

Xynnn007 commented Oct 15, 2024

mythi commented Oct 15, 2024

mythi commented Oct 15, 2024

Xynnn007 commented Oct 15, 2024

Xynnn007 commented Oct 15, 2024

mythi commented Oct 15, 2024 • edited Loading

Xynnn007 commented Oct 16, 2024

mythi commented Oct 16, 2024

Xynnn007 commented Oct 16, 2024

mythi commented Oct 16, 2024

Xynnn007 commented Oct 16, 2024 • edited Loading

mythi Oct 16, 2024

Choose a reason for hiding this comment

Xynnn007 Oct 17, 2024

Choose a reason for hiding this comment

Xynnn007 Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

mythi Oct 23, 2024

Choose a reason for hiding this comment

mythi commented Oct 16, 2024

Xynnn007 commented Oct 17, 2024

mythi commented Oct 17, 2024

Xynnn007 commented Oct 17, 2024

mythi commented Oct 17, 2024

Xynnn007 commented Oct 21, 2024 • edited Loading

fitzthum commented Oct 22, 2024 • edited Loading

mythi commented Oct 23, 2024

jialez0 left a comment

Choose a reason for hiding this comment

mythi commented Oct 23, 2024

Xynnn007 commented Oct 23, 2024 • edited Loading

mythi commented Oct 23, 2024 • edited Loading

Xynnn007 commented Oct 23, 2024

mythi commented Oct 23, 2024

Xynnn007 commented Oct 9, 2024 •

edited

Loading

Xynnn007 commented Oct 10, 2024 •

edited

Loading

mythi commented Oct 15, 2024 •

edited

Loading

Xynnn007 commented Oct 16, 2024 •

edited

Loading

Xynnn007 Oct 23, 2024 •

edited

Loading

Xynnn007 commented Oct 21, 2024 •

edited

Loading

fitzthum commented Oct 22, 2024 •

edited

Loading

Xynnn007 commented Oct 23, 2024 •

edited

Loading

mythi commented Oct 23, 2024 •

edited

Loading